Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove rustfmt bug workaround #6083

Closed
wants to merge 1 commit into from
Closed

Remove rustfmt bug workaround #6083

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Sep 25, 2020

rust-lang/rustfmt#1873 was fixed earlier this year.

changelog: none

@rust-highfive
Copy link

r? @yaahc

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 25, 2020
@ghost
Copy link
Author

ghost commented Sep 25, 2020

I haven't tested this Windows. I can rely on CI to do that, right?

@matthiaskrgr
Copy link
Member

matthiaskrgr commented Sep 25, 2020

I think bors will try on windows, too?
@bors try

@bors
Copy link
Collaborator

bors commented Sep 25, 2020

⌛ Trying commit c8b9093 with merge f42f7a0...

bors added a commit that referenced this pull request Sep 25, 2020
Remove rustfmt bug workaround

rust-lang/rustfmt#1873 was fixed earlier this year.

changelog: none
@bors
Copy link
Collaborator

bors commented Sep 25, 2020

☀️ Try build successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Build commit: f42f7a0 (f42f7a087b59fa8018216f6fe5ab0bdf1a8f56fb)

@bors
Copy link
Collaborator

bors commented Sep 25, 2020

⌛ Trying commit c8b9093 with merge b323bb7...

bors added a commit that referenced this pull request Sep 25, 2020
Remove rustfmt bug workaround

rust-lang/rustfmt#1873 was fixed earlier this year.

changelog: none
@matthiaskrgr
Copy link
Member

Argh, apparently editing a comment that contains @bors try will retrigger bors, sorry :/

@bors
Copy link
Collaborator

bors commented Sep 25, 2020

☀️ Try build successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Build commit: b323bb7 (b323bb73ae9d201e09179a486c88994d3b576c7d)

@matthiaskrgr
Copy link
Member

Hmm, it seems the rustfmt commit 5e667d326f929a0770c1e56cd0a921dfb16400e2 has not yet landed inside the rustfmt submodule of the rustc repo.

@flip1995
Copy link
Member

bors only runs the fmt test on the ubuntu runner. But this test (Clippy Dev Test) is also run in the PR CI.

Temporary this can be changed by removing this line:

After that the fmt test is run as part of the test suite in Clippy Test (bors) on the Windows runner.

@flip1995
Copy link
Member

flip1995 commented Oct 1, 2020

r? @matthiaskrgr

reassigning this, since @yaahc stepped down as a reviewer to focus on the error handling project group

@rust-highfive rust-highfive assigned matthiaskrgr and unassigned yaahc Oct 1, 2020
@ebroto ebroto assigned ebroto and unassigned matthiaskrgr Oct 8, 2020
@ebroto
Copy link
Member

ebroto commented Oct 8, 2020

@bors try

(let's try flip1995's suggestion)

@bors
Copy link
Collaborator

bors commented Oct 8, 2020

⌛ Trying commit 7822e6c with merge ca31ddf...

bors added a commit that referenced this pull request Oct 8, 2020
Remove rustfmt bug workaround

rust-lang/rustfmt#1873 was fixed earlier this year.

changelog: none
@bors
Copy link
Collaborator

bors commented Oct 8, 2020

💔 Test failed - checks-action_test

@ebroto
Copy link
Member

ebroto commented Oct 8, 2020

@bors try

(missing rustfmt component)

@bors
Copy link
Collaborator

bors commented Oct 8, 2020

⌛ Trying commit fded020 with merge a9045b7...

bors added a commit that referenced this pull request Oct 8, 2020
Remove rustfmt bug workaround

rust-lang/rustfmt#1873 was fixed earlier this year.

changelog: none
@bors
Copy link
Collaborator

bors commented Oct 8, 2020

💔 Test failed - checks-action_test

@ebroto
Copy link
Member

ebroto commented Oct 8, 2020

It seems we hit a similar error as in #3118, will try something like #3701 later

@ebroto
Copy link
Member

ebroto commented Oct 18, 2020

Sorry for leaving this unaddressed, there has been a spike in PRs to review and I did not have much time/energy to fight the CI/Windows tandem.

I'll try to find some time in the next couple days, but I would say this is low priority for now.

@bors
Copy link
Collaborator

bors commented Dec 11, 2020

☔ The latest upstream changes (presumably #6401) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@ebroto
Copy link
Member

ebroto commented Dec 18, 2020

Sorry for having left this aside for so long. 🙏

I think I've managed to reproduce the original error running this on CI on my fork:

error: couldn't read \\?\D:\a\rust-clippy\rust-clippy\tests\ui\..\auxiliary\test_macro.rs: The filename, directory name, or volume label syntax is incorrect. (os error 123)
    Finished dev [unoptimized + debuginfo] target(s) in 23.21s

     Running `clippy_dev/target\debug\clippy_dev.exe fmt --check`
Error writing files: failed to resolve mod `test_macro`: \\?\D:\a\rust-clippy\rust-clippy\tests\ui\..\auxiliary\test_macro.rs does not exist
error: couldn't read \\?\D:\a\rust-clippy\rust-clippy\tests\ui\..\auxiliary\test_macro.rs: The filename, directory name, or volume label syntax is incorrect. (os error 123)
rustfmt failed on D:\a\rust-clippy\rust-clippy\tests\ui\implicit_hasher.rs


Error writing files: failed to resolve mod `test_macro`: \\?\D:\a\rust-clippy\rust-clippy\tests\ui\..\auxiliary\test_macro.rs does not exist
Formatting check failed.
rustfmt failed on D:\a\rust-clippy\rust-clippy\tests\ui\implicit_hasher.rs
Run `cargo dev fmt` to update formatting.

error: process didn't exit successfully: `clippy_dev/target\debug\clippy_dev.exe fmt --check` (exit code: 1)

CI logs

It looks like the bug the workaround fixes, right?

@ebroto ebroto added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Dec 18, 2020
@ghost
Copy link
Author

ghost commented Dec 20, 2020

Yeah, that looks like it. I guess we can close this again and try in another year.

@ghost ghost closed this Dec 20, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants